Move contracts to a new Contract model
#12094
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
garyhtou
reviewed
Nov 17, 2025
sampoder
reviewed
Nov 23, 2025
sampoder
approved these changes
Nov 23, 2025
Member
sampoder
left a comment
sampoder
left a comment
There was a problem hiding this comment.
lgtm! minus comment on type of contract
garyhtou
approved these changes
Nov 25, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 26, 2025
## Summary of the problem <!-- Why are these changes being made? What problem does it solve? Link any related issues to provide more details. --> https://appsignal.com/hack-club/sites/6596247683eb67648f30f807/exceptions/incidents/2394 Bug introduced in #12094 - since `Contract` and `OrganizerPositionInvite` are now decoupled, there are some fields of the `Contract` model that need to be set using info from the `Contractable`. Notably, the `prefills` jsonb includes event data that must be included in the `Contract` before creation, since it is used in the after create callback that sends the payload to Docuseal. While we were doing this in the `send_contract` method on `OrganizerPositionInvite`, we were not doing this when creating a contract using the create action in `ContractsController` (used when sending a contract on a pre-existing invite or organizer position), causing the above bug. ## Describe your changes <!-- Explain your thought process to the solution and provide a quick summary of the changes. --> While this could have been fixed by just grabbing the necessary data from the `Contractable` and passing it to `Contract.new` in `ContractsController`, it's better to maintain that contracts should only be created using methods on the `Contractable`. This way, changes can be easily made to what data is passed to the `Contract`, and any extra code (such as updating the `OrganizerPosition` or Airtable can be done in one place. This does involve updating the `OrganizerPositionInvite` after it has been sent and accepted - I've fixed a validation to not fail when this happens, but it would also be possible to modify the code so that this model is essentially "frozen" after being accepted, and only update the `is_signee` field before accepting.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of the problem
Currently, contracts are in an
OrganizerPosition::Contractmodel and tightly coupled toOrganizerPositionInvite. With the new application flow I'm working on, we'll need contracts to be abstracted more to support a source from either anEvent::ApplicationorOrganizerPositionInvite, sinceOrganizerPositionInvitewill not exist yet when the applicant is signing the contract during the application flow.Describe your changes
Primarily, this PR adds a new
Contractmodel that is mostly the same asOrganizerPosition::Contract, with a couple differences:Contractableinstead of a relationship directly toOrganizerPositionInvite. OnlyOrganizerPositionInviteimplements this right now, but in the applications PREvent::Applicationwill also implement it.typecolumn instead of apurposecolumn. Thepurposecolumn was added in the original implementation ofOrganizerPosition::Contract, but it is not currently being used. This PR adds this back as STI, and a future PR will implement better automation for termination contractsAll code that references
OrganizerPosition::Contracthas been changed to referenceContractinstead, including the relations onUserandEvent. A future PR will drop theOrganizerPosition::Contractmodel and associated table.In the database migration that creates this new table, steps have been added that copy over current data from the
organizer_position_contractstable and renames item types for versions stored in the PaperTrailversionstable.I've tested this PR using my own DocuSeal account in test mode, but it should be tested with our production DocuSeal account and a real contract before merging into production.